-
Notifications
You must be signed in to change notification settings - Fork 158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
removed v from version of oauth2-proxy deployment.yaml #189
removed v from version of oauth2-proxy deployment.yaml #189
Conversation
@RadOctocode Thanks for your contribution, but some work remains to be considered. |
@RadOctocode Eventually, can you link me to some documents explaining the rationale behind this change and the technical reasons for this best practice? |
Hi Pierluigi, sorry about the wait. I'm happy to make the changes to the other files and bumping the version to remove the v from the helm chart. As far as the reasoning for removing the implicit v from the helm chart, many companies host their images in private docker repositories that have rules. As part of a financial institution one of those rules for our private registry is that image semver flags are not prefixed with a v. |
fa0c3c2
to
3ba8edd
Compare
Hey @pierluigilenoci I removed the stripping of v from the helper function as well, please let me know if there is anything else I should do. |
@RadOctocode, the chart needs a version bump. |
3ba8edd
to
8722c72
Compare
@pierluigilenoci I bumped the chart up by one minor version, let me know if I should change this to be a patch bump instead or if I should add something to artifacthub.io/changes |
@RadOctocode, could you please also edit the annotation for ArtifactHub? |
…rsion bump Signed-off-by: RadOctocode <[email protected]>
8722c72
to
d055307
Compare
@pierluigilenoci I have edited the annotation for artifacthub |
Reasoning: assuming all versions for the deployment image for oauth2-proxy will start with v is bad practice.